Skip to content

Conversation

@18202781743
Copy link
Contributor

@18202781743 18202781743 commented Jul 9, 2025

  1. Changed BubbleNotifyItem to AppNotifyItem in NotifyStagingModel for
    better naming consistency
  2. Simplified DataAccessorProxy by removing redundant source validity
    checks
  3. Improved entity routing logic with clearer conditions
  4. Added proper source initialization in DataAccessorProxy constructor
  5. Enhanced removeEntity() to check entity validity before removal
  6. Streamlined clear() and removeEntityByApp() to only operate on source

The changes improve code maintainability and reduce redundant
checks while maintaining the same functionality. The naming change
from BubbleNotifyItem to AppNotifyItem better reflects the item's
purpose, and the data access logic simplification makes the code more
straightforward and less error-prone.

refactor: 更新通知处理和数据访问逻辑

  1. 在 NotifyStagingModel 中将 BubbleNotifyItem 改为 AppNotifyItem 以提高
    命名一致性
  2. 通过移除冗余的源有效性检查简化了 DataAccessorProxy
  3. 使用更清晰的条件改进了实体路由逻辑
  4. 在 DataAccessorProxy 构造函数中添加了适当的源初始化
  5. 增强了 removeEntity() 在删除前检查实体有效性
  6. 简化了 clear() 和 removeEntityByApp() 使其仅操作源数据

这些变更提高了代码可维护性,在保持相同功能的同时减少了冗余检查。从
BubbleNotifyItem 到 AppNotifyItem 的命名更改更好地反映了项目的用途,数据
访问逻辑的简化使代码更加直接且不易出错。

pms: BUG-305735

@sourcery-ai
Copy link

sourcery-ai bot commented Jul 9, 2025

Reviewer's Guide

Refactor DataAccessorProxy to consistently manage and route operations through m_source by initializing it properly, removing redundant validity checks, and simplifying CRUD return logic; and update NotifyStagingModel to replace BubbleNotifyItem with AppNotifyItem for notification creation and initialization.

Class diagram for NotifyStagingModel notification item refactor

classDiagram
    class NotifyStagingModel {
        - QList<AppNotifyItem*> m_appNotifies
        + void remove(qint64 id)
        + void open()
    }
    class AppNotifyItem {
        + AppNotifyItem(NotifyEntity entity)
    }
    %% BubbleNotifyItem is removed from usage
    %% Previously: class BubbleNotifyItem { + BubbleNotifyItem(NotifyEntity entity) }
    NotifyStagingModel --> AppNotifyItem : uses
Loading

Class diagram for DataAccessorProxy CRUD logic refactor

classDiagram
    class DataAccessorProxy {
        - DataAccessor* m_impl
        - DataAccessor* m_source
        + addEntity(entity)
        + replaceEntity(id, entity)
        + updateEntityProcessedType(id, type)
        + fetchEntity(id)
        + fetchEntityCount(appName, type)
        + fetchLastEntity(appName, type)
        + fetchEntities(appName, type, maxCount)
        + fetchApps(maxCount)
        + removeEntity(id)
        + removeEntityByApp(appName)
        + clear()
        + setSource(source)
    }
    class DataAccessor {
        + addEntity(entity)
        + replaceEntity(id, entity)
        + updateEntityProcessedType(id, type)
        + fetchEntity(id)
        + fetchEntityCount(appName, type)
        + fetchLastEntity(appName, type)
        + fetchEntities(appName, type, maxCount)
        + fetchApps(maxCount)
        + removeEntity(id)
        + removeEntityByApp(appName)
        + clear()
        + isValid()
    }
    class MemoryAccessor {
        + isValid()
    }
    DataAccessorProxy --> DataAccessor : uses as m_impl
    DataAccessorProxy --> DataAccessor : uses as m_source
    MemoryAccessor --|> DataAccessor
Loading

File-Level Changes

Change Details Files
Streamline DataAccessorProxy source management and routing logic
  • Initialize m_source in singleton instance alongside m_impl
  • Add null guard and delete existing m_source in setSource
  • Remove redundant m_source validity checks in add/replace/fetch/count methods
  • Simplify conditional returns and default fallback for source operations
panels/notification/common/dataaccessorproxy.cpp
Replace BubbleNotifyItem with AppNotifyItem in NotifyStagingModel
  • Instantiate AppNotifyItem instead of BubbleNotifyItem on row insertion
  • Use AppNotifyItem for initial notification list in open() loop
panels/notification/center/notifystagingmodel.cpp

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @18202781743 - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

1. Changed BubbleNotifyItem to AppNotifyItem in NotifyStagingModel for
better naming consistency
2. Simplified DataAccessorProxy by removing redundant source validity
checks
3. Improved entity routing logic with clearer conditions
4. Added proper source initialization in DataAccessorProxy constructor
5. Enhanced removeEntity() to check entity validity before removal
6. Streamlined clear() and removeEntityByApp() to only operate on source

The changes improve code maintainability and reduce redundant
checks while maintaining the same functionality. The naming change
from BubbleNotifyItem to AppNotifyItem better reflects the item's
purpose, and the data access logic simplification makes the code more
straightforward and less error-prone.

refactor: 更新通知处理和数据访问逻辑

1. 在 NotifyStagingModel 中将 BubbleNotifyItem 改为 AppNotifyItem 以提高
命名一致性
2. 通过移除冗余的源有效性检查简化了 DataAccessorProxy
3. 使用更清晰的条件改进了实体路由逻辑
4. 在 DataAccessorProxy 构造函数中添加了适当的源初始化
5. 增强了 removeEntity() 在删除前检查实体有效性
6. 简化了 clear() 和 removeEntityByApp() 使其仅操作源数据

这些变更提高了代码可维护性,在保持相同功能的同时减少了冗余检查。从
BubbleNotifyItem 到 AppNotifyItem 的命名更改更好地反映了项目的用途,数据
访问逻辑的简化使代码更加直接且不易出错。

pms: BUG-305735
@deepin-ci-robot
Copy link

deepin pr auto review

代码审查意见:

  1. 代码修改

    • NotifyStagingModel类中,removeopen函数中将BubbleNotifyItem替换为AppNotifyItem,需要确认这一改动是否符合业务逻辑需求。
    • DataAccessorProxy类中,setSource函数中增加了对source参数的空指针检查,这是一个好的做法,可以避免潜在的空指针异常。
  2. 代码质量

    • DataAccessorProxy类中,setSource函数中删除了旧的source对象后,应该调用delete而不是delete[],因为m_source是一个指针,而不是一个数组。
    • DataAccessorProxy类中,addEntity函数中,如果m_source为空,应该返回一个错误码(例如-1),而不是继续执行。这可以避免在后续代码中处理无效的实体。
    • DataAccessorProxy类中,updateEntityProcessedType函数中,如果m_source为空,应该返回一个错误码(例如-1),而不是继续执行。这可以避免在后续代码中处理无效的实体。
    • DataAccessorProxy类中,fetchEntityfetchEntityCountfetchLastEntityfetchEntities函数中,如果m_source为空,应该返回一个空实体或空列表,而不是继续执行。这可以避免在后续代码中处理无效的数据。
  3. 代码性能

    • DataAccessorProxy类中,fetchEntityfetchEntityCountfetchLastEntityfetchEntities函数中,每次调用m_source的方法时,都进行了isValid检查。如果m_source经常为空,可以考虑在初始化时进行一次检查,并将结果存储在一个成员变量中,以避免重复检查。
  4. 代码安全性

    • DataAccessorProxy类中,setSource函数中,在删除旧的source对象后,应该调用delete而不是delete[],因为m_source是一个指针,而不是一个数组。这可以避免潜在的内存泄漏。

综上所述,代码修改是合理的,但需要确保这些修改符合业务逻辑需求,并且对代码质量、性能和安全性进行了适当的考虑。

@18202781743 18202781743 changed the title fix: replace BubbleNotifyItem with AppNotifyItem refactor: update notification handling and data access logic Jul 9, 2025
@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, yixinshark

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@18202781743 18202781743 merged commit 3f362d3 into linuxdeepin:master Jul 10, 2025
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants